-
Notifications
You must be signed in to change notification settings - Fork 12
Add unified diff support for luatest.assert_equals
#438
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
69c5c84 to
fef0177
Compare
ligurio
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Max, thanks for the patch!
Please take a look on my comments.
1d91b35 to
26138ab
Compare
edcc8eb to
6a22390
Compare
6a22390 to
f2c9536
Compare
ligurio
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
This patch adds unified diff output for `t.assert_equals()` failures, using a vendored Lua implementation of google/diff-match-patch (`luatest/vendor/diff_match_patch.lua` taken from [^1]). Closes tarantool#412 [^1]: https://github.com/google/diff-match-patch/blob/master/lua/diff_match_patch.lua
f2c9536 to
8f7455d
Compare
| local used_line_diff = true | ||
|
|
||
| if diffs == nil then | ||
| diffs = diff_match_patch.diff_main(expected_text, actual_text) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like there's no test for this fallback mode - if I remove this code, the tests still pass.
Why can't we compute a line-based diff in all cases? Is there any point in char-based diff? Does it look good?
| same: nested-shared | ||
| s: qux | ||
| - n: 10 | ||
| dec: '1.23' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The value of dec is different (1.23 vs 3.21). Why isn't it reported here? BTW do you take into account that the order of keys in a YaML document is undefined? Need to sort it?
This patch adds unified diff output for
t.assert_equals()failures when the test suite is run with the--diffoption, using a vendored Lua implementation of google/diff-match-patch (luatest/vendor/diff_match_patch.luataken from 1).Closes #412
Here is the output for the example from the issue:
code
Footnotes
https://github.com/google/diff-match-patch/blob/master/lua/diff_match_patch.lua ↩